closes #53 - add influxDB #244
Conversation
Codecov Report
@@ Coverage Diff @@
## master #244 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 14 +1
Lines 494 540 +46
Branches 55 63 +8
=========================================
+ Hits 494 540 +46
Continue to review full report at Codecov.
|
database/influx/influx.js
Outdated
| throw new Error(`failed to create new influx user with ${username} name`); | ||
| } | ||
| }; | ||
| influxModule.deleteAccount = async (account) => { |
There was a problem hiding this comment.
This function only uses username, so why is the entire account object needed?
lib/util.js
Outdated
| if (arangoDbExists) await arango.deleteAccount(username); | ||
|
|
||
| const influxDbExists = await influx.checkAccount(username); | ||
| if (influxDbExists) await influx.deleteAccount(user); |
There was a problem hiding this comment.
You have the choice to pass in username, so you should do that instead of user. I don't think it makes sense to pass in an object to a function that only uses one string data-type variable as an input. That is confusing -- it makes me think that the function needs something else from user other than just user.username
There was a problem hiding this comment.
Basically, this code threw me off. PG and Elastic use username for checks and user for deletions while Arango needs only username in both cases. I was trying to be consistent so I added user as argument for Influx.
const pgDbExists = await pg.userHasPgAccount(username);
if (pgDbExists) await pg.deletePgAccount(user);
const esDbExists = await es.checkAccount(username);
if (esDbExists) await es.deleteAccount(user);
const arangoDbExists = await arango.checkIfDatabaseExists(username);
if (arangoDbExists) await arango.deleteAccount(username);
I changed it, so deleteAccount now needs only username as argument.
src/server.js
Outdated
| let app = null; | ||
|
|
||
| const arangoModule = require("../database/arango/arango"); | ||
| const influxModule = require("../database/influx/influx.js"); |
There was a problem hiding this comment.
You don't need the .js when importing a JS file
2 participants
InfluxDB was chosen over TimescaleDB mostly because latter requires older versions of Postgres and maintaining multiply psqls on the same machine is rather annoying. It's also more popular at least for now.
Since this PR adds new influxDB button welcome page snapshots were updated.
To safely merge:
localhostand8086Influx: "PRODUCTION INFLUXDB HOST",needs to be changed to whatever host will be used in productionProofreading of influxDB tutorial is appreciated.